CreateSpark #124
CreateSpark #124
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a createSpark() factory method that provides a unified interface for creating either SparkMax or SparkFlex motor controllers based on the MotorConfig parameter. This simplifies client code by allowing dynamic selection of the appropriate Spark controller type without needing to explicitly call createSparkMax() or createSparkFlex().
Changes:
- Added two overloaded
createSpark()methods that dispatch to the appropriatecreateSparkMax()orcreateSparkFlex()method based on themotorConfig.controllerType - The first overload uses default Spark configuration, while the second accepts a custom
SparkBaseConfig
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case SPARK_FLEX: | ||
| return createSparkFlex(id, motorConfig, config); | ||
| default: | ||
| return null; |
There was a problem hiding this comment.
The default case returns null, which could lead to NullPointerExceptions if an unexpected controllerType is passed. While this pattern is consistent with other methods in this class (e.g., getConfigAccessor at line 157-158, createConfig at line 161-171, getControllerType at line 180), consider whether it would be safer to throw an IllegalArgumentException with a descriptive message explaining that the controllerType is unsupported. This would make debugging easier and fail faster if an invalid configuration is used.
| return null; | |
| throw new IllegalArgumentException( | |
| "Unsupported controllerType in MotorControllerFactory.createSpark: " | |
| + motorConfig.controllerType); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@CoolSpy3 could you review pls |
src/test/java/org/carlmontrobotics/lib199/MotorControllerFactoryTest.java
Outdated
Show resolved
Hide resolved
…reateSpark creates the right motor type
|
@CoolSpy3 Done 👍 |
CoolSpy3
left a comment
CoolSpy3
left a comment
There was a problem hiding this comment.
Great; thanks! This looks good ✅ Just fix the javadoc errors. (I think the problem might be that the ...s are causing Javadoc to look for varargs declarations that don't exist, but idk for sure.)
CoolSpy3
left a comment
CoolSpy3
left a comment
There was a problem hiding this comment.
Nvm. Sorry, GitHub was showing me the not-most-recent commit :P
2 participants
This add a createSpark() method. It returns a SparkBase initialized to a SparkMax/Flex depending on the motorConfig passed in.